Skip to content

Fix missing owners for path error to include path#8450

Merged
JimSuplizio merged 3 commits intoAzure:mainfrom
JimSuplizio:MissingOwnersForPath
Jun 14, 2024
Merged

Fix missing owners for path error to include path#8450
JimSuplizio merged 3 commits intoAzure:mainfrom
JimSuplizio:MissingOwnersForPath

Conversation

@JimSuplizio
Copy link
Copy Markdown
Contributor

@JimSuplizio JimSuplizio commented Jun 14, 2024

The problem right now is that the missing owners error, There are no owners defined for CODEOWNERS entry., is too generic. At the moment, the only missing owners in any of the repositories running the linter are for source path/owner lines. The fix here is to add the path to the error in order to make it distinct.

There is one big issue here:
Monikers can also have missing owners but because we can't put line numbers into the baseline file, for obvious reasons, there's no good differentiation for monikers missing errors.
The mitigation for the issue:
The only repositories that have this error in their baseline files are azure-sdk-for-java and azure-sdk-for-js. For Java, this error is actually because of two source path/owner lines lacking owners. For JS, this error was apparently fixed and never removed from the baseline file. When this change goes in, and the linter version is updated, Java's baseline file will need to be updated with the new errors, which will include the path. This means that any Moniker's missing owners will fail the pipeline at the PR level since these don't currently exist in any repository running the linter.

This change only affects the linter. The parser, which is used by a number of tools, remains unchanged. Only the linter version is going to need to be updated when this is merged.

@JimSuplizio JimSuplizio added the CODEOWNERS Linter Anything related to the CODEOWNERS linter label Jun 14, 2024
@JimSuplizio JimSuplizio self-assigned this Jun 14, 2024
@JimSuplizio JimSuplizio requested a review from weshaggard June 14, 2024 17:08
Copy link
Copy Markdown
Member

@weshaggard weshaggard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One suggestion but otherwise seems reasonable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CODEOWNERS Linter Anything related to the CODEOWNERS linter

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants